test(utils): add unit tests for ValidatorStatus and getValidatorStatus#298
test(utils): add unit tests for ValidatorStatus and getValidatorStatus#298lodekeeper-z wants to merge 2 commits into
Conversation
Add 12 unit tests covering: - All 9 validator lifecycle states (pending_initialized through withdrawal_done) - toString/fromString roundtrip - matchesCategory for all 4 categories (pending, active, exited, withdrawal) - Invalid category returns false - Invalid string returns null 🤖 Generated with AI assistance
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces unit testing for the validator status logic, which previously lacked test coverage. By implementing a suite of tests covering the full validator lifecycle and utility methods, this change ensures the reliability of the validator status state machine, which is critical for upcoming NAPI bindings and related functionality. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a suite of unit tests for validator status functions and methods in validator_status.zig. The review feedback identifies several style guide violations in the test implementation, including the need for in-place initialization of large structs via out-pointers, adherence to the 100-character line limit, and the use of descriptive variable names instead of abbreviations.
| fn makeValidator( | ||
| activation_eligibility_epoch: u64, | ||
| activation_epoch: u64, | ||
| exit_epoch: u64, | ||
| withdrawable_epoch: u64, | ||
| slashed: bool, | ||
| effective_balance: u64, | ||
| ) Validator.Type { | ||
| return .{ | ||
| .pubkey = [_]u8{0} ** 48, | ||
| .withdrawal_credentials = [_]u8{0} ** 32, | ||
| .effective_balance = effective_balance, | ||
| .slashed = slashed, | ||
| .activation_eligibility_epoch = activation_eligibility_epoch, | ||
| .activation_epoch = activation_epoch, | ||
| .exit_epoch = exit_epoch, | ||
| .withdrawable_epoch = withdrawable_epoch, | ||
| }; | ||
| } | ||
|
|
||
| test "getValidatorStatus - pending_initialized" { | ||
| // activation_epoch > current_epoch AND activation_eligibility_epoch == FAR_FUTURE | ||
| const v = makeValidator(constants.FAR_FUTURE_EPOCH, constants.FAR_FUTURE_EPOCH, constants.FAR_FUTURE_EPOCH, constants.FAR_FUTURE_EPOCH, false, 32_000_000_000); | ||
| try testing.expectEqual(ValidatorStatus.pending_initialized, getValidatorStatus(&v, 100)); | ||
| } |
There was a problem hiding this comment.
This new test code has a few violations of the repository's style guide:
- In-place initialization for large structs:
Validator.Typeis a large struct. Helper functions that create instances of it, likemakeValidator, should use an out-pointer to initialize the struct in-place, rather than returning it by value. - Line length: Function calls with many arguments, like the calls to
makeValidator, exceed the 100-column limit and should be formatted across multiple lines. - No abbreviated variable names: The variable
vis an abbreviation. The style guide requires descriptive names likevalidator.
Please apply these changes throughout the new test code. Here is an example of how makeValidator and the first test should be refactored:
fn makeValidator(
target: *Validator.Type,
activation_eligibility_epoch: u64,
activation_epoch: u64,
exit_epoch: u64,
withdrawable_epoch: u64,
slashed: bool,
effective_balance: u64,
) void {
target.* = .{
.pubkey = [_]u8{0} ** 48,
.withdrawal_credentials = [_]u8{0} ** 32,
.effective_balance = effective_balance,
.slashed = slashed,
.activation_eligibility_epoch = activation_eligibility_epoch,
.activation_epoch = activation_epoch,
.exit_epoch = exit_epoch,
.withdrawable_epoch = withdrawable_epoch,
};
}
test "getValidatorStatus - pending_initialized" {
// activation_epoch > current_epoch AND activation_eligibility_epoch == FAR_FUTURE
var validator: Validator.Type = undefined;
makeValidator(
&validator,
constants.FAR_FUTURE_EPOCH,
constants.FAR_FUTURE_EPOCH,
constants.FAR_FUTURE_EPOCH,
constants.FAR_FUTURE_EPOCH,
false,
32_000_000_000,
);
try testing.expectEqual(ValidatorStatus.pending_initialized, getValidatorStatus(&validator, 100));
}
References
- Construct larger structs in-place by passing an out pointer during initialization. The Validator.Type struct is large, so it should be initialized in-place to avoid moving the object, which can lead to undesirable stack growth. (Lines 318-325, 326-351) (link)
- Hard limit all line lengths to at most 100 columns. To wrap a function call, add a trailing comma and run
zig fmt. (Lines 400-403) (link) - Do not abbreviate variable names. Use descriptive names to improve code clarity. (Line 221) (link)
Remove extra blank line caught by CI fmt check. 🤖 Generated with AI assistance
2 participants
Summary
Add 12 unit tests for
validator_status.zigcovering the complete validator lifecycle state machine and utility functions.Tests added
Motivation
validator_status.zighad 0 test coverage. It's used by the NAPIgetValidatorStatusbinding and will be used by the upcominggetValidatorsByStatusmethod.🤖 Generated with AI assistance